Conversation
MaziyarMK
commented
Jun 10, 2023
- offer request owner (airline) logo
- offer request slice segment stops
Add missing fields to Airline dataclass
ulissesalmeida
left a comment
There was a problem hiding this comment.
Looks pretty good to me.
I added a few comments to keep the file consistent.
jesse-c
left a comment
There was a problem hiding this comment.
LGTM aside from needing the test fixtures updated! Thanks for your work on this, @MaziyarMK.
| id: str | ||
| name: str | ||
| iata_code: Optional[str] | ||
| logo_lockup_url: Optional[str] |
There was a problem hiding this comment.
test: Could you please add examples of these to the test fixture? [1][2]
[1]
duffel-api-python/tests/fixtures/get-airlines.json
Lines 3 to 7 in 3f01a35
[2]
{
"iata_code": "BA",
"id": "aln_00001876aqC8c5umZmrRds",
"name": "British Airways",
"logo_lockup_url": "https://assets.duffel.com/img/airlines/for-light-background/full-color-lockup/BA.svg",
"logo_symbol_url": "https://assets.duffel.com/img/airlines/for-light-background/full-color-logo/BA.svg"
}
| """Additional segment-specific information about the stops, if any, included in the segment""" | ||
|
|
||
| id: str | ||
| duration: str |
There was a problem hiding this comment.
test: Similar request here to add some stops to the test fixture [1].
[1]
duffel-api-python/tests/fixtures/get-offers.json
Lines 135 to 208 in 3f01a35
| id: str | ||
| name: str | ||
| iata_code: Optional[str] | ||
| logo_lockup_url: Optional[str] |
There was a problem hiding this comment.
future: We need to add conditions_of_carriage_url [1] as well.
[1] https://duffel.com/docs/api/v1/airlines/schema#airlines-schema-conditions-of-carriage-url.
|
|
||
| @dataclass | ||
| class OfferSliceSegmentStop: | ||
| """Additional segment-specific information about the stops, if any, included in the segment""" |
There was a problem hiding this comment.
fix: This line is too long [1]. You can fix it by running tox -e linting or flake8 locally.
[1] https://github.com/duffelhq/duffel-api-python/actions/runs/5245627223/jobs/9476358835?pr=287
| arriving_at=get_and_transform( | ||
| json, "arriving_at", parse_datetime | ||
| ), | ||
| departing_at=get_and_transform( | ||
| json, "departing_at", parse_datetime | ||
| ), |
There was a problem hiding this comment.
fix: The reason for the static analysis (pyright) failures is that this helper function, get_and_transform, is for when values are optional.
Those 2 aren't, so you can parse them directly.
| arriving_at=get_and_transform( | |
| json, "arriving_at", parse_datetime | |
| ), | |
| departing_at=get_and_transform( | |
| json, "departing_at", parse_datetime | |
| ), | |
| arriving_at=parse_datetime(json["arriving_at"]), | |
| departing_at=parse_datetime(json["departing_at"]), |